Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not validate if the command is 'NewLine' #640

Merged
merged 3 commits into from
Jul 15, 2022

Conversation

sigmaSd
Copy link
Contributor

@sigmaSd sigmaSd commented Jul 13, 2022

NewLine is in my experience used when the user actually want to force a newline regardless of the result of the validation.

This pr exempt newline from validation.

demo: (I first hit enter each time then ctrl-s)
rl

@gwenn
Copy link
Collaborator

gwenn commented Jul 14, 2022

Related to #379.

@gwenn
Copy link
Collaborator

gwenn commented Jul 14, 2022

I would prefer that we don't call validate in this case.
Something like:

diff --git a/src/command.rs b/src/command.rs
index b46c7d5..c16f966 100644
--- a/src/command.rs
+++ b/src/command.rs
@@ -25,6 +25,16 @@ pub fn execute<H: Helper>(
 ) -> Result<Status> {
     use Status::{Proceed, Submit};

+    match cmd {
+        Cmd::EndOfFile | Cmd::AcceptLine | Cmd::AcceptOrInsertLine { .. } | Cmd::Newline => {
+            if s.has_hint() || !s.is_default_prompt() {
+                // Force a refresh without hints to leave the previous
+                // line as the user typed it after a newline.
+                s.refresh_line_with_msg(None)?;
+            }
+        }
+        _ => {}
+    };
     match cmd {
         Cmd::CompleteHint => {
             complete_hint_line(s)?;
@@ -58,11 +68,6 @@ pub fn execute<H: Helper>(
             s.edit_overwrite_char(c)?;
         }
         Cmd::EndOfFile => {
-            if s.has_hint() || !s.is_default_prompt() {
-                // Force a refresh without hints to leave the previous
-                // line as the user typed it after a newline.
-                s.refresh_line_with_msg(None)?;
-            }
             if s.line.is_empty() {
                 return Err(error::ReadlineError::Eof);
             } else if !input_state.is_emacs_mode() {
@@ -119,12 +124,10 @@ pub fn execute<H: Helper>(
                 kill_ring.kill(&text, Mode::Append);
             }
         }
-        Cmd::AcceptLine | Cmd::AcceptOrInsertLine { .. } | Cmd::Newline => {
-            if s.has_hint() || !s.is_default_prompt() {
-                // Force a refresh without hints to leave the previous
-                // line as the user typed it after a newline.
-                s.refresh_line_with_msg(None)?;
-            }
+        Cmd::Newline => {
+            s.edit_insert('\n', 1)?;
+        }
+        Cmd::AcceptLine | Cmd::AcceptOrInsertLine { .. } => {
             let validation_result = s.validate()?;
             let valid = validation_result.is_valid();
             let end = s.line.is_end_of_input();
@@ -140,8 +143,7 @@ pub fn execute<H: Helper>(
                 ) => {
                     return Ok(Submit);
                 }
-                (Cmd::Newline, ..)
-                | (Cmd::AcceptOrInsertLine { .. }, false, _)
+                (Cmd::AcceptOrInsertLine { .. }, false, _)
                 | (Cmd::AcceptOrInsertLine { .. }, true, false) => {
                     if valid || !validation_result.has_message() {
                         s.edit_insert('\n', 1)?;

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Jul 14, 2022

done

@gwenn gwenn merged commit c45de92 into kkawakam:master Jul 15, 2022
@gwenn
Copy link
Collaborator

gwenn commented Jul 15, 2022

Thanks.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Jul 15, 2022

Would you mind releasing a new version? I want to add this to deno repl

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Jul 15, 2022

Thanks!

@gwenn
Copy link
Collaborator

gwenn commented Jul 17, 2022

Version 10.0.0 released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants